Rebase with develop by shibu-kv · Pull Request #304 · rdkcentral/telemetry

shibu-kv · 2026-03-30T21:18:59Z

No description provided.

…rm calls (#287) * RDKB-63722:Analyze and fix/mitigate memory leaks from curl_easy_perform calls Reason for change: Three compounding sources of unbounded internal state in OpenSSL / libcurl that code failed to constrain: 1] Unbounded connection cache per handle — CURLOPT_MAXCONNECTS was never set. libcurl defaults to 5 cached connection objects per handle. Each cached entry retains a live SSL object (TLS state, session ticket, handshake records). On devices with 180-day uptime where the server resets connections every 30s + each connection having probability of hitting different load balancers, this cache churns and accumulates stale SSL objects over time. 2] SSL session ticket accumulation — CURLOPT_SSL_SESSIONID_CACHE was never disabled. libcurl maintains a session-ticket cache inside each handle's SSL_CTX. Stale tickets from rotated or expired server certs or load balancers remain cached until OpenSSL's 300s session timeout. Across days of uptime with endpoint or cert changes, this accumulates visibly in VmRSS. 3] OpenSSL per-thread error queue not drained— OpenSSL uses a per-thread ERR_STATE list. Every TLS operation that encounters an error (connection reset, cert verify failure, network timeout) pushes records onto this list. The libcurl code never called ERR_clear_error() explicitly, so on any error path the queue has chances to grow indefinitely 4] set CURLOPT_DNS_CACHE_TIMEOUT=30 to bound DNS cache lifetime to the server reset interval, preventing stale IP->hostname mappings from accumulating as load balancer IPs rotate. Test Procedure: please refer the ticket comments Risks: Medium * Addressed the L1 Testcases Compilation Error with respect to multicurlinterface.c * Added the OPENSSL_thread_stop() at the end of worker thread * Reduce Memory Fragmentation to limit the send/receive buffer to 8KB

…sl version (#291) * RDKB-63722: Update multicurlinterface.c * RDKB-63722: Build fix * RDKB-63722: Build fix * astyle formatter error changes are addressed --------- Co-authored-by: tabbas651 <thamimrazith_abbasali@comcast.com>

…284) * RDKB-63834: Reject and Remove Corrupted Config Files In Persistance Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> * RDKB-63834: Add unit tests for empty config file detection and fprintf failure paths in persistence.c (#285) * Initial plan * Add unit tests for empty config file detection and fprintf failure in persistence.c Co-authored-by: shibu-kv <89052442+shibu-kv@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: shibu-kv <89052442+shibu-kv@users.noreply.github.com> --------- Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: shibu-kv <89052442+shibu-kv@users.noreply.github.com>

…ration (#295) * Modify the set_logdemand value as true for all scenarios except LOG_UPLOAD * Formatting the code changes * Modify the misleading function and variable names

#299) * RDKEMW-15233:[SERXIONE-8445/XIONE-18418] Develop Support Branch Integration Signed-off-by: PriyaDharshini_Kathiravan <priyakathiravan05@gmail.com> * Update source/bulkdata/reportprofiles.c Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add debug messages for seekmap and marker values --------- Signed-off-by: PriyaDharshini_Kathiravan <priyakathiravan05@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR rebases onto develop and incorporates multiple fixes and refactors around persistence handling, scheduler seekmap retention logic, and HTTP/OpenSSL memory behavior, with corresponding unit test updates.

Changes:

Skip and delete empty persisted config files; add write-failure handling in saveConfigToFile().
Replace get/set_logdemand with get/set_retainseekmap across scheduler and tests.
Add curl option tuning and OpenSSL per-thread cleanup calls; link with -lcrypto and update tests/build files accordingly.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
source/utils/persistence.c	Skips empty config files and handles `fprintf()` failures by removing the partially written profile.
source/test/utils/UtilsTest.cpp	Adds tests for empty config file handling and `fprintf()` failure/success paths.
source/test/scheduler/schedulerTest.cpp	Updates scheduler flag tests to new `retainseekmap` API.
source/test/mocks/FileioMock.cpp	Adds `__fprintf_chk` interception for FORTIFY builds.
source/test/dcautils/Makefile.am	Adds `-lcrypto` for unit test link compatibility.
source/test/bulkdata/SchedulerMock.h / .cpp	Renames mock methods to `retainseekmap`.
source/test/bulkdata/profileTest.cpp	Updates expectation to `set_retainseekmap`.
source/test/bulkdata/Makefile.am	Adds `-lcrypto` for test binaries.
source/telemetry2_0.c	Updates signal handling to use `set_retainseekmap` (but on-demand path needs adjustment).
source/scheduler/scheduler.h / .c	Renames flag APIs and alters default/interrupt logic (currently introduces behavioral regressions).
source/protocol/http/multicurlinterface.c	Adds curl memory-bounding opts and OpenSSL thread cleanup/error-queue clearing.
source/protocol/http/Makefile.am	Links `libhttp` with `-lcrypto`.
source/dcautil/dca.c	Adds additional debug logs for seek value and marker extraction.
source/bulkdata/reportprofiles.c	Moves `T2InitProperties()` earlier; removes on-demand flag set (needs restoration via new API).
CHANGELOG.md	Adds release entries; contains a new typo.
.gitignore	Adds autotools/build artifact ignores.

Copilot · 2026-03-30T21:25:53Z

source/scheduler/scheduler.c

 static pthread_mutex_t scMutex;
 static bool sc_initialized = false;
-static bool islogdemand = false;
+static bool isretainSeekmap = true;


isretainSeekmap default was changed from false (previous islogdemand) to true. With current TimeoutThread() logic, this makes the else branch (which passes isClearSeekMap=true) effectively unreachable and changes behavior for LOG_UPLOAD interrupts (seekmap would be retained by default). To preserve previous semantics, initialize this flag to false and only set it to true for on-demand uploads where seekmap must be retained.

Suggested change

static bool isretainSeekmap = true;

static bool isretainSeekmap = false;

Copilot · 2026-03-30T21:25:54Z

source/scheduler/scheduler.c

+                if (get_retainseekmap() == true)
                {
-                    set_logdemand(false);
-                    timeoutNotificationCb(tProfile->name, false);
+                    timeoutNotificationCb(tProfile->name, false); // Passing clearseekvalue as false
                }
                else
                {
-                    timeoutNotificationCb(tProfile->name, true);
+                    set_retainseekmap(true); //After triggering LOG upload resetting the retainseekmap value to true so the next report generation doesn't affect
+                    timeoutNotificationCb(tProfile->name, true); //Passing clearseek value as true


In the interrupt path, when get_retainseekmap() is true you never reset it back to the default state (the previous code did set_logdemand(false) after consuming the on-demand flag). This causes subsequent non-on-demand LOG_UPLOAD interrupts to incorrectly retain seekmap. Consider resetting the flag after handling the retain case, and avoid setting it to true in the branch that calls timeoutNotificationCb(..., true) (that branch represents the clear-seekmap case).

Copilot · 2026-03-30T21:25:54Z

source/telemetry2_0.c

        }
        else if (sig == LOG_UPLOAD_ONDEMAND || sig == SIGIO)
        {
            T2Info(("LOG_UPLOAD_ONDEMAND received!\n"));


The LOG_UPLOAD_ONDEMAND/SIGIO signal handler no longer sets the scheduler flag used to retain the seekmap (previously set_logdemand(true)). As a result, on-demand uploads may clear/retain seekmap based on stale state rather than the signal type. Set set_retainseekmap(true) (or equivalent) in this branch before interrupting report generation.

Suggested change

T2Info(("LOG_UPLOAD_ONDEMAND received!\n"));

T2Info(("LOG_UPLOAD_ONDEMAND received!\n"));

set_retainseekmap(true);

Copilot · 2026-03-30T21:25:54Z

source/bulkdata/reportprofiles.c

@@ -428,7 +428,6 @@ static void* reportOnDemand(void *input)
    if(!strncmp(action, ON_DEMAND_ACTION_UPLOAD, MAX_PROFILENAMES_LENGTH))
    {
        T2Info("Upload XCONF report on demand \n");


reportOnDemand() previously set the scheduler flag (was set_logdemand(true)) before generating an on-demand report. With that removed, on-demand uploads may no longer retain seekmap as intended (and behavior will depend on whatever global flag value happens to be set). Set set_retainseekmap(true) (or otherwise pass the intent) before calling generateDcaReport(false, true).

Suggested change

T2Info("Upload XCONF report on demand \n");

T2Info("Upload XCONF report on demand \n");

set_retainseekmap(true);

Copilot · 2026-03-30T21:25:55Z

source/protocol/http/multicurlinterface.c

+        // Limit the send buffer to 8KB (Requires libcurl 7.62.0+)
+        CURL_SETOPT_CHECK(pool_entries[i].easy_handle, CURLOPT_UPLOAD_BUFFERSIZE, 8192L);


CURLOPT_UPLOAD_BUFFERSIZE is not available in older libcurl versions (as noted in the comment) and this call is currently unguarded. This will cause compile failures on platforms with older libcurl headers. Please wrap this option in an #ifdef CURLOPT_UPLOAD_BUFFERSIZE guard (similar to how CURLOPT_TCP_KEEPCNT is handled).

Suggested change

// Limit the send buffer to 8KB (Requires libcurl 7.62.0+)

CURL_SETOPT_CHECK(pool_entries[i].easy_handle, CURLOPT_UPLOAD_BUFFERSIZE, 8192L);

// Limit the send buffer to 8KB (Requires libcurl 7.62.0+)

#ifdef CURLOPT_UPLOAD_BUFFERSIZE

CURL_SETOPT_CHECK(pool_entries[i].easy_handle, CURLOPT_UPLOAD_BUFFERSIZE, 8192L);

#endif

Copilot · 2026-03-30T21:25:55Z

source/protocol/http/multicurlinterface.c

+#include <openssl/err.h>
+#include <openssl/crypto.h>


This file now includes and calls OpenSSL-specific APIs (<openssl/err.h>, OPENSSL_thread_stop, ERR_clear_error) unconditionally. The build system currently does not appear to detect/configure an OpenSSL dependency (no OpenSSL/libcrypto checks in configure.ac), and libcurl can be built against non-OpenSSL TLS backends. Please add compile-time guards and an Autotools feature check (e.g., AC_CHECK_HEADERS/AC_CHECK_LIB or pkg-config) so builds without OpenSSL headers/libcrypto still work (either by disabling this cleanup path or using a curl-backend-agnostic approach).

Suggested change

#include <openssl/err.h>

#include <openssl/crypto.h>

/*

* OpenSSL usage in this file is optional. We attempt to include the headers

* only if they are available; otherwise we provide no-op fallbacks so that

* builds without OpenSSL headers/libs still compile and link.

*/

#if defined(__has_include)

# if __has_include(<openssl/err.h>) && __has_include(<openssl/crypto.h>)

# include <openssl/err.h>

# include <openssl/crypto.h>

# define T2_HAVE_OPENSSL 1

# endif

#endif

#ifndef T2_HAVE_OPENSSL

#define T2_HAVE_OPENSSL 0

/* No-op fallbacks when OpenSSL is not available */

static inline void t2_openssl_thread_stop_fallback(void)

{

/* Intentionally empty: OpenSSL not available */

}

static inline void t2_openssl_err_clear_fallback(void)

{

/* Intentionally empty: OpenSSL not available */

}

/* Map OpenSSL APIs to local no-op fallbacks when headers are absent. */

#ifndef OPENSSL_thread_stop

#define OPENSSL_thread_stop() t2_openssl_thread_stop_fallback()

#endif

#ifndef ERR_clear_error

#define ERR_clear_error() t2_openssl_err_clear_fallback()

#endif

#endif /* !T2_HAVE_OPENSSL */

Copilot · 2026-03-30T21:25:55Z

source/protocol/http/Makefile.am


 libhttp_la_SOURCES = curlinterface.c multicurlinterface.c
-libhttp_la_LDFLAGS = -shared -fPIC -lcurl
+libhttp_la_LDFLAGS = -shared -fPIC -lcurl -lcrypto


-lcrypto is added unconditionally to libhttp_la_LDFLAGS, but configure.ac does not currently check for libcrypto/OpenSSL headers. This can break cross-compiles or platforms where libcurl is not built on OpenSSL (or OpenSSL dev headers/libs aren’t present). Consider making libcrypto linkage conditional on a configure-time check and only enabling the OpenSSL-specific code paths when that check succeeds.

Suggested change

libhttp_la_LDFLAGS = -shared -fPIC -lcurl -lcrypto

libhttp_la_LDFLAGS = -shared -fPIC -lcurl

Copilot · 2026-03-30T21:25:56Z

CHANGELOG.md


+#### [1.8.5](https://github.com/rdkcentral/telemetry/compare/1.8.4...1.8.5)
+
+- RDKEMW-15233: Previous log look up initialiation corrections [`#299`](https://github.com/rdkcentral/telemetry/pull/299)


Spelling: “initialiation” → “initialization”.

Suggested change

- RDKEMW-15233: Previous log look up initialiation corrections [`#299`](https://github.com/rdkcentral/telemetry/pull/299)

- RDKEMW-15233: Previous log look up initialization corrections [`#299`](https://github.com/rdkcentral/telemetry/pull/299)

Copilot · 2026-03-30T21:25:56Z

source/test/scheduler/schedulerTest.cpp

+TEST(GET_RETAINSEEKMAP, TEST1)
 {
-   EXPECT_EQ( false, get_logdemand());
+   EXPECT_EQ( true, get_retainseekmap());
 }

-TEST(SET_LOGDEMAND, TEST1)
+TEST(SET_RETAINSEEKMAP, TEST1)
 {
-   set_logdemand(true);
+   set_retainseekmap(true);
 }


This test asserts the default is true, which matches the new isretainSeekmap = true initialization in scheduler.c. If retainseekmap is intended to preserve the previous logdemand semantics (default false, set true only for on-demand uploads), this expectation should be updated to false once the scheduler default is corrected.

tabbas651 and others added 13 commits March 18, 2026 16:05

Changelog updates for release 1.8.2

28b5426

Merge branch 'release/1.8.2' into develop

0492bec

Changelog updates for 1.8.3 release

cac6d7f

Merge branch 'release/1.8.3' into develop

f3ca322

RDKEMW-15233 [SERXIONE-8445/XIONE-18418] Develop Support Branch Integ…

36848b6

…ration (#295) * Modify the set_logdemand value as true for all scenarios except LOG_UPLOAD * Formatting the code changes * Modify the misleading function and variable names

Changelog updates for 1.8.4 release

a336d80

Merge branch 'release/1.8.4' into develop

57b7a6e

Changelog updates for release 1.8.5

3d23970

Merge branch 'release/1.8.5' into develop

31778cc

shibu-kv requested a review from a team as a code owner March 30, 2026 21:19

Copilot AI review requested due to automatic review settings March 30, 2026 21:19

Copilot started reviewing on behalf of shibu-kv March 30, 2026 21:19 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

shibu-kv deleted the branch feature/robust-thread-synchroniation March 30, 2026 21:34

shibu-kv closed this Mar 30, 2026

github-actions bot locked and limited conversation to collaborators Mar 30, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rebase with develop#304

Rebase with develop#304
shibu-kv wants to merge 13 commits intofeature/robust-thread-synchroniationfrom
develop

shibu-kv commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	static bool isretainSeekmap = true;
	static bool isretainSeekmap = false;

	T2Info(("LOG_UPLOAD_ONDEMAND received!\n"));
	T2Info(("LOG_UPLOAD_ONDEMAND received!\n"));
	set_retainseekmap(true);

	T2Info("Upload XCONF report on demand \n");
	T2Info("Upload XCONF report on demand \n");
	set_retainseekmap(true);

		// Limit the send buffer to 8KB (Requires libcurl 7.62.0+)
		CURL_SETOPT_CHECK(pool_entries[i].easy_handle, CURLOPT_UPLOAD_BUFFERSIZE, 8192L);

-#include <openssl/err.h>
-#include <openssl/crypto.h>
+/*
+ * OpenSSL usage in this file is optional. We attempt to include the headers
+ * only if they are available; otherwise we provide no-op fallbacks so that
+ * builds without OpenSSL headers/libs still compile and link.
+ */
+#if defined(__has_include)
+#  if __has_include(<openssl/err.h>) && __has_include(<openssl/crypto.h>)
+#    include <openssl/err.h>
+#    include <openssl/crypto.h>
+#    define T2_HAVE_OPENSSL 1
+#  endif
+#endif
+#ifndef T2_HAVE_OPENSSL
+#define T2_HAVE_OPENSSL 0
+/* No-op fallbacks when OpenSSL is not available */
+static inline void t2_openssl_thread_stop_fallback(void)
+{
+    /* Intentionally empty: OpenSSL not available */
+}
+static inline void t2_openssl_err_clear_fallback(void)
+{
+    /* Intentionally empty: OpenSSL not available */
+}
+/* Map OpenSSL APIs to local no-op fallbacks when headers are absent. */
+#ifndef OPENSSL_thread_stop
+#define OPENSSL_thread_stop() t2_openssl_thread_stop_fallback()
+#endif
+#ifndef ERR_clear_error
+#define ERR_clear_error() t2_openssl_err_clear_fallback()
+#endif
+#endif /* !T2_HAVE_OPENSSL */

	libhttp_la_LDFLAGS = -shared -fPIC -lcurl -lcrypto
	libhttp_la_LDFLAGS = -shared -fPIC -lcurl


		#### [1.8.5](https://github.com/rdkcentral/telemetry/compare/1.8.4...1.8.5)

		- RDKEMW-15233: Previous log look up initialiation corrections [`#299`](https://github.com/rdkcentral/telemetry/pull/299)

Conversation

shibu-kv commented Mar 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants